Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allowing empty log group prefixes #87

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

ryyakobe
Copy link
Contributor

In some RFDK constructs we allow setting a custom log group prefix.

If no prefix is given, then we use a default prefix. This is done with the trenary operator ?::
const prefix = props.logGroupProps?.logGroupPrefix ? props.logGroupProps.logGroupPrefix : SOME_DEFAULT_PREFIX;

So if the prefix is empty logGroupPrefix = '', we will still use the default prefix, instead of using no prefix at all. This PR fixes this issue by replacing the trenary operator ?: with ??:
const prefix = props.logGroupProps?.logGroupPrefix ?? SOME_DEFAULT_PREFIX;

Also, updated the unit-tests.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@ddneilson ddneilson self-requested a review August 27, 2020 18:52
@ddneilson ddneilson added the contribution/core This is a PR that came from AWS. label Aug 27, 2020
Copy link
Contributor

@ddneilson ddneilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together, Roman

Copy link
Contributor

@yashda yashda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks Roman!

@ddneilson ddneilson merged commit e53571c into aws:mainline Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants